Implemented rfft!, irfft! and brfft!
#222
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The new functions work with a new `PaddedRFFTArray` type. They also work with appropriately sized Compex Arrays and Real Subarrays.
…ed to solve ambiguity. Same for `brfft!(f::PaddedRFFTArray, i::Integer)`
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #222 +/- ##
==========================================
+ Coverage 62.67% 68.14% +5.46%
==========================================
Files 5 6 +1
Lines 434 565 +131
==========================================
+ Hits 272 385 +113
- Misses 162 180 +18 ☔ View full report in Codecov by Sentry. |
stevengj
reviewed
Oct 23, 2021
stevengj
reviewed
Oct 23, 2021
favba
commented
Oct 26, 2021
The transformation was not really in-place.
…exOrRealReinterpretArray`
…rrays. - `PaddedRFFTArray` now accepts any `AbstractArray` type, provided the strides are continuous. - Changed some variable names to be consistent with other functions (`d` is always the length of the first dimension of the logical real array)
|
Is there any progress on this PR? This would be very useful for us to have in TimeseriesSurrogates.jl, where performance of some of our methods would be improved if the in-place inverse transform |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR supersedes #54 .
Most of the code is the same.
It provides the same features but adds the capability of calling
irfft!(c::Array{Complex},d::Integer)andrfft!(r::SubArray{<:Real}which are implicitly converted toPaddedRFFTArray. This makes it much simpler to convert code usingrfftandirfftto use the in-place versions of thoseIn order to implement those features, we must allow
PaddedRFFTArrayto be initialized with a complex array, in which case the real view of the array must be reinterpreted out the complex array.Although there is a way to make reinterpreted real-to-complex array performant (see Tim Holy's comment), it doesn't seem the same trick can be used to make reinterpreted complex-to-real arrays performant.
In this PR I'm using an internal
ComplexOrRealReinterpretArraythat relies on loading and storing directly from pointers to the raw data, whileGC.@preserveing appropriately the underlying data.To Do:
rfft!(a::PaddedRFFTArray,region=1:ndims(a))andrfft!(a::DenseArray{<:Complex},d::Integer)in a better way